-
Notifications
You must be signed in to change notification settings - Fork 302
Fixing an issue with using show configuration within extensions #15766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing an issue with using show configuration within extensions #15766
Conversation
adb1ba4 to
62de1e9
Compare
62de1e9 to
ce9bcb5
Compare
I resolved this issue. It turns out that dynamic importing appears to be a problem in harvester and I just decided to work around it since it's not necessary for show configuration. harvester-show.mp4Just to preempt a possible concern. We don't show the config tab due to 63d2bcc#diff-65d145eecdf6d47052fa05eb10aad1c81026a913269c7b60ce8da494292dc0a9R41-R44 . We may want to revisit this but I don't think it needs to be changed now. |
ce9bcb5 to
2bacc73
Compare
| export function importDrawer(name) { | ||
| return () => undefined; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't impact anything in harvester. This is necessary if we want dynamic importing to work in the future.
…roken in harvester
2bacc73 to
34ac80c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally with harvester standalone and rancher integration mode.
Show configuration button work well on both modes. Thanks for the fix.



Summary
Fixes #15658
Technical notes summary
This is caused because the extension has copied an old version of the resource-class which doesn't include show configuration.
This will now check to see if it exists and if it doesn't it will use the latest definition and bind the resource to it.
Areas or cases that should be tested
Show configuration, show labels and annotations in the masthead.
Just a note, if you refresh the detail page the new detail page will not be rendered. I commented here rancher/kubewarden-ui#1333 (comment)
Areas which could experience regressions
Show configuration, show labels and annotations in the masthead.
Screenshot/Video
show-configuration-extension.mp4
Checklist
Admin,Standard UserandUser Base